Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix duplicate testing when unique suite name #114

Merged
merged 3 commits into from
Nov 4, 2016

Conversation

vjeffrey
Copy link

@vjeffrey vjeffrey commented Nov 3, 2016

fixes #109

trying to work on a test for this....

puts 'config inspec tests', config[:inspec_tests]
# if check for duplicates returns true, only send back the value for config[:inspec_tests]
return config[:inspec_tests] if check_for_duplicates(local_suite_files, config[:inspec_tests])

# get local tests and get run list of profiles
(local_suite_files + config[:inspec_tests]).compact
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should simply use (local_suite_files + config[:inspec_tests]).compact.uniq? We also need to double-check that this works with the new complex objects

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local suite files comes in with the full path:

** local suite files
/Users/vjeffrey/test-kitchen-inspec/test/recipes/master
## config[:inspec_tests]
{:path=>"test/recipes/master"}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, shoot, the complex objects isn't working well. darnit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, maybe we do the following:
1.) we rewrite local suite files to return a complex object like

{:path=>"/Users/vjeffrey/test-kitchen-inspec/test/recipes/master"}

2.) We also resolve the inspec_tests to absolute paths before we hand them over to inspec
3.) Extend your check_for_duplicates to only work with objects?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@vjeffrey vjeffrey force-pushed the vj/fix-testing-duplicates branch 8 times, most recently from e16c318 to f8b2fcb Compare November 4, 2016 00:29
@vjeffrey
Copy link
Author

vjeffrey commented Nov 4, 2016

k, so as part of this change, since we are harmonizing profile targets, there's an error that is raised if the inspec tests that are specified are not specified in the defined format (like path: path/to/file or url: url). i was initially trying to support backwards compatibility, but it was getting complicated with transforming the hashes....so i went this route. if that's too much, we can work around to support non-hashes....but it seems like since we're pushing this hash change across audit cookbook and here, it would make sense to enforce it. The error message is nice and verbose so the user should understand pretty quickly...

@chris-rock
Copy link
Collaborator

@Vj I think we stay backwards compatible for now (easier upgrade for users), but we do duplicate checking only for the new syntax.

Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
Signed-off-by: Christoph Hartmann <chris@lollyrock.com>
Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
@chris-rock
Copy link
Collaborator

Nice work @vjeffrey

@chris-rock chris-rock merged commit 0fe5f74 into master Nov 4, 2016
@chris-rock chris-rock deleted the vj/fix-testing-duplicates branch November 4, 2016 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate testing when verifier specified in suite definition
2 participants